Cleanup angular listeners #265
Conversation
Signed-off-by: Merit <merit@flatfile.io>
|
✅ No documentation updates required. |
WalkthroughBoth the SpaceFrame and Space components were updated to implement the Changes
Sequence Diagram(s)sequenceDiagram
participant SF as SpaceFrame Component
participant ML as Message Listener
SF->>ML: Check for existing listener
ML-->>SF: Remove listener (if exists)
note over SF: New message listener is initialized
SF->>SF: ngOnDestroy()
SF->>ML: Call removeMessageListener (if set)
sequenceDiagram
participant S as Space Component
participant EF as EffectRef
participant SS as SpaceService
S->>EF: Setup effect (with isDestroyed check)
note over S: Component initialization
S->>S: ngOnDestroy()
S->>EF: Call destroy()
S->>SS: Call closeEmbed()
Suggested reviewers
✨ Finishing Touches
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/angular/angular-sdk/src/lib/sdk/space-frame/spaceFrame.component.ts (1)
37-38: Well-structured private properties for resource tracking.The addition of:
removeMessageListenerfor storing the cleanup functionisDestroyedflag for tracking component stateThese properties improve resource management and help prevent memory leaks.
Note: The
isDestroyedflag is defined but not currently used in this component. Consider using it in key methods or removing it if unnecessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/angular/angular-sdk/src/lib/sdk/space-frame/spaceFrame.component.ts(5 hunks)packages/angular/angular-sdk/src/lib/sdk/space.component.ts(4 hunks)
🧰 Additional context used
🧬 Code Definitions (2)
packages/angular/angular-sdk/src/lib/sdk/space-frame/spaceFrame.component.ts (2)
packages/angular/angular-sdk/src/lib/sdk/space-close-modal/spaceCloseModal.component.ts (1)
SpaceCloseModalPropsType(3-10)packages/javascript/src/listener.ts (2)
createListener(24-51)createSimpleListener(53-109)
packages/angular/angular-sdk/src/lib/sdk/space.component.ts (1)
packages/angular/angular-sdk/src/lib/sdk/space-frame/spaceFrame.component.ts (1)
SpaceFramePropsType(11-17)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Lint
- GitHub Check: Test
🔇 Additional comments (14)
packages/angular/angular-sdk/src/lib/sdk/space.component.ts (6)
1-1: Appropriate import additions for lifecycle management.Adding OnDestroy and EffectRef imports is necessary for implementing proper lifecycle management and cleanup in Angular components.
11-11: Good implementation of OnDestroy interface.Implementing OnDestroy allows the component to properly clean up resources when it's destroyed, preventing memory leaks and unintended side effects.
25-26: Well-structured private properties for resource management.These private properties enhance component lifecycle management:
effectRefenables proper cleanup of effectsisDestroyedflag helps prevent updates after component destructionThis is a good practice for managing component state during destruction.
29-42: Improved effect handling with destruction guard.The effect implementation now includes:
- Storing the effect reference for later cleanup
- Added a guard condition to prevent updates after component destruction
- Refactored space initialization logic for better readability
This implementation follows Angular best practices for reactive programming with effects.
56-60: Updated handleCloseInstance to include closeEmbed call.The method now correctly calls
spaceService.closeEmbed()when closing the instance, ensuring proper resource cleanup.
62-65: Proper implementation of ngOnDestroy.The ngOnDestroy method correctly:
- Cleans up the effect by calling
effectRef.destroy()- Ensures embedded resources are properly released with
spaceService.closeEmbed()This implementation follows Angular best practices for component cleanup.
packages/angular/angular-sdk/src/lib/sdk/space-frame/spaceFrame.component.ts (8)
1-1: Appropriate import of OnDestroy interface.Adding OnDestroy to the imports allows the component to implement proper lifecycle cleanup.
24-24: Good implementation of OnDestroy interface.Implementing OnDestroy allows the component to properly clean up resources and event listeners when it's destroyed.
43-46: Improved listener management with cleanup.Adding cleanup for any existing listener before creating a new one prevents memory leaks and duplicate event handlers.
52-59: Proper storage and cleanup of message listener.The message listener creation now properly:
- Stores the returned cleanup function in
removeMessageListener- Uses the stored function for cleanup in the callback
- Uses optional chaining for safe function calls
This implementation ensures proper resource management.
61-73: Consistent listener management for both code paths.The alternative listener creation path now follows the same pattern as the primary path, ensuring consistent cleanup regardless of which code path is taken.
86-86: Added listener cleanup to handleConfirm method.Calling
removeMessageListener?.()in the handleConfirm method ensures that the message listener is properly cleaned up when the dialog is confirmed.
109-110: Simplified token assignment.The token assignment has been streamlined by directly using
spaceFrameProps.localAccessToken, making the code more concise.
125-125: Proper cleanup in ngOnDestroy.The ngOnDestroy method now correctly calls
removeMessageListener?.()to clean up the message listener when the component is destroyed.
2 participants
Please explain how to summarize this PR for the Changelog:
Before
https://www.loom.com/share/1d50487e96064a59a1aaf8754929f373
After
https://www.loom.com/share/8006f60e182d477f84fb01edc6fdc458
Tell code reviewer how and what to test: